Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

test(pageserver): add test keyspace into collect_keyspace #8016

Merged
merged 2 commits into from
Jun 12, 2024
Merged

test(pageserver): add test keyspace into collect_keyspace #8016

merged 2 commits into from
Jun 12, 2024

Conversation

skyzh
Copy link
Member

@skyzh skyzh commented Jun 11, 2024

Problem

Some test cases add random keys into the timeline, but it is not part of the collect_keyspace, this will cause compaction remove the keys.

Summary of changes

The pull request adds a field to supply extra keyspaces during unit tests.

Checklist before requesting a review

  • I have performed a self-review of my code.
  • If it is a core feature, I have added thorough tests.
  • Do we need to implement analytics? if so did you add the relevant metrics to the dashboard?
  • If this PR requires public announcement, mark it with /release-notes label and add several sentences in this section.

Checklist before merging

  • Do not forget to reformat commit message to not include the above checklist

@skyzh
test(pageserver): add test keyspace into collect_keyspace
4a40b2e 
Signed-off-by: Alex Chi Z <chi@neon.tech>
@skyzh skyzh requested a review from problame June 11, 2024 19:21
@skyzh skyzh requested a review from a team as a code owner June 11, 2024 19:21
@github-actions GitHub Actions
Copy link

github-actions bot commented Jun 11, 2024 •
edited
Loading

3234 tests run: 3092 passed, 0 failed, 142 skipped (full report)

Flaky tests (7)

Postgres 16

  • test_secondary_background_downloads: debug
  • test_vm_bit_clear_on_heap_lock_blackbox: debug

Postgres 15

  • test_ondemand_wal_download_in_replication_slot_funcs: release
  • test_storage_controller_smoke: debug
  • test_subscriber_restart: debug

Postgres 14

  • test_storage_controller_smoke: debug
  • test_vm_bit_clear_on_heap_lock_blackbox: release

Code coverage* (full report)

  • functions: 31.6% (6631 of 20990 functions)
  • lines: 48.6% (51503 of 106058 lines)

* collected from Rust tests only

The comment gets automatically updated with the latest test results
62d732a at 2024-06-12T17:51:40.450Z :recycle:

problame
problame approved these changes Jun 12, 2024
View reviewed changes
Copy link
Contributor

@problame problame left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not clear enough from the comments why you called it extra_test_dense_keyspace as opposed to extra_test_keyspace

What's the context for this change? AuxFilesV2 or the new compaction algorithm?

pageserver/src/tenant.rs Show resolved Hide resolved
@skyzh
Copy link
Member Author

skyzh commented Jun 12, 2024

collect_keyspace returns two values: dense keyspace and sparse keyspace. For sparse keyspace, the test cases can simply use AUX keys for testing; for dense keyspace, users should add their keyspace into the extra keyspace. Probably I should add both extra_dense_ks and extra_sparse_ks, but given it's not needed now, I don't plan to add both of them in this pull request.

@skyzh
resolve comments
62d732a 
Signed-off-by: Alex Chi Z <chi@neon.tech>
@skyzh skyzh force-pushed the skyzh/test-keyspace branch from f8f2f1f to 62d732a Compare June 12, 2024 15:50
@skyzh
Copy link
Member Author

skyzh commented Jun 12, 2024

What's the context for this change? AuxFilesV2 or the new compaction algorithm?

Fixing some existing test cases that might be flaky b/c the keys could be garbage-collected, and also make the compaction algorithm tests able to use the data keyspace to do WAL replays.

@skyzh skyzh enabled auto-merge (squash) June 12, 2024 17:30
@skyzh skyzh merged commit 836d1f4 into main Jun 12, 2024
63 of 64 checks passed
@skyzh skyzh deleted the skyzh/test-keyspace branch June 12, 2024 17:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@koivunej koivunej koivunej left review comments

@problame problame problame approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@skyzh @problame @koivunej